-
Notifications
You must be signed in to change notification settings - Fork 51
feat(web): remove reminder from day and week views #1402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…d reminder handling - Updated `Header` component to conditionally render the `Reminder` based on the `showReminder` prop, allowing for more flexible UI rendering. - Refactored `useReminderHotkey` to accept an `enabled` parameter, enabling or disabling the hotkey functionality based on the reminder state. - Modified `MemoryRouter` utility to accept `initialEntries` for better routing control in tests. - Removed the `Edit Reminder` option from the command palette and related tests to streamline the user interface and focus on essential functionalities.
…est.tsx - Deleted tests related to reminder functionality in the Calendar form, including keyboard shortcuts and command palette interactions, to streamline the test suite and focus on essential features. - Removed unused import from DayCmdPalette.tsx to clean up the codebase.
- Removed the `measurements` prop from `SidebarDraftProvider` in `CalendarView`, streamlining its usage. - Updated the `SidebarDraftProvider` component to reflect the removal of the `measurements` prop, enhancing clarity and reducing complexity.
- Updated the `customRender` function to include a `CustomWrapper` option, allowing for more flexible component wrapping during tests. - Simplified the wrapper logic by creating a new `Wrapper` component that conditionally includes `CustomWrapper` or defaults to `BaseProviders`, improving test setup clarity.
- Removed local date utility from event mocks to standardize date handling. - Updated `EUROPE_TRIP` event dates to fixed values for consistency in tests. - Introduced a new `timer.baktest.ts` file with comprehensive tests for the Timer class, covering initialization, error handling, and event emissions. - Enhanced `state.weekEvents` mock to reflect accurate event counts and success states, improving test reliability. - Refactored `store.test.util.ts` to simplify state type definitions, ensuring better compatibility with mock data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR removes the reminder functionality from Day and Week views while retaining it in the Now view. The changes enhance the user experience by conditionally displaying reminders based on the view context. Additionally, the PR includes significant test infrastructure improvements, including better mock state management, custom wrapper support in test utilities, and comprehensive Timer class tests.
- Introduces a
showReminderprop to the Header component for conditional reminder display - Updates the reminder hotkey hook to accept an
enabledparameter for conditional activation - Removes "Edit Reminder" commands from Day and Week view command palettes
- Enhances test infrastructure with custom wrapper support and improved mock state definitions
Reviewed changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/views/Now/view/NowView.tsx | Passes showReminder={true} to Header to keep reminder in Now view |
| packages/web/src/views/Now/view/NowView.test.tsx | Updates test expectations for route handling and shortcut counts |
| packages/web/src/views/Day/view/DayViewContent.tsx | Passes showReminder={false} to Header and adds explicit void return type |
| packages/web/src/views/Day/view/DayViewContent.test.tsx | Updates test to verify reminder is not rendered in Day view |
| packages/web/src/views/Day/components/Header/Header.tsx | Adds showReminder prop with conditional rendering logic |
| packages/web/src/views/Day/components/Header/Header.test.tsx | Adds test coverage for both showReminder states |
| packages/web/src/views/Day/components/DayCmdPalette.tsx | Removes "Edit Reminder" command from Day view palette |
| packages/web/src/views/Day/components/DayCmdPalette.test.tsx | Removes tests for deleted "Edit Reminder" functionality |
| packages/web/src/views/CmdPalette/CmdPalette.tsx | Removes "Edit Reminder" command from general command palette |
| packages/web/src/views/Calendar/hooks/shortcuts/useFocusHotkey.ts | Adds enabled parameter to conditionally activate reminder hotkey |
| packages/web/src/views/Calendar/components/Sidebar/Sidebar.render.test.tsx | Refactors test to use direct component rendering with custom providers |
| packages/web/src/views/Calendar/components/Header/Header.tsx | Removes reminder component and related imports from Week view header |
| packages/web/src/views/Calendar/components/Draft/sidebar/context/SidebarDraftProvider.tsx | Removes unused measurements parameter and React import |
| packages/web/src/views/Calendar/Calendar.tsx | Updates SidebarDraftProvider to remove measurements prop |
| packages/web/src/views/Calendar/Calendar.form.test.tsx | Removes all reminder-related tests |
| packages/web/src/tests/utils/state/store.test.util.ts | Adds union types with any for flexible test state definitions |
| packages/web/src/tests/utils/providers/MemoryRouter.tsx | Adds support for initialEntries parameter |
| packages/web/src/tests/mocks/state/state.weekEvents.ts | Updates mock state with accurate counts, success flags, and additional state slices |
| packages/web/src/tests/mocks/mock.render.tsx | Adds support for custom wrapper components in tests |
| packages/core/src/util/timer.baktest.ts | Introduces comprehensive Timer class tests (note: disabled via .baktest extension) |
| packages/core/src/mocks/v1/events/events.misc.ts | Replaces dynamic date generation with fixed dates for EUROPE_TRIP event |
- Removed unused event states from `state.weekEvents` mock to simplify the structure. - Updated `store.test.util.ts` to improve type definitions for test state, ensuring better compatibility with mock data. - Added `importGCal` state to `sync` in `preloadedState` for improved testing scenarios. - Enhanced type safety in `Sidebar.render.test.tsx` by specifying types for `dateCalcs` and `measurements` props.
Co-authored-by: Copilot <[email protected]>
- Removed unused imports and streamlined mock setup in `DayCmdPalette.test.tsx` to enhance test clarity and maintainability. - Simplified the test structure by eliminating unnecessary dependencies, improving overall test performance.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 20 out of 21 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
packages/web/src/views/Day/components/DayCmdPalette.test.tsx:21
- The comment "... (rest of mocks)" is not informative and doesn't add value. If this comment is meant as a placeholder, it should either be removed or replaced with actual documentation about what the mocks do.
__esModule: true,
- Updated the `Header` component to utilize `useCallback` for the reminder focus handler, enhancing performance by memoizing the function. - Simplified the hotkey functionality to conditionally focus the reminder based on the `showReminder` prop, improving code clarity and maintainability.
EUROPE_TRIPevent dates to fixed values for consistency in tests.timer.baktest.tsfile with comprehensive tests for the Timer class, covering initialization, error handling, and event emissions.state.weekEventsmock to reflect accurate event counts and success states, improving test reliability.store.test.util.tsto simplify state type definitions, ensuring better compatibility with mock data.Closes #1400